Skip to content

Conversation

pblazej
Copy link
Contributor

@pblazej pblazej commented Oct 9, 2025

Turns out that the configuration closure:

  • must be run before any logs (expected) - this is kinda fixable
  • may run once per process, so will break for anyone using the same framework... and we cannot do any default setup

If we need the flexibility anyway, maybe it's better to drop the swift-logging right now...

My assupmtions:

  • we use pure OSLog by default (info or warning)
  • we can easily enable rtc internal logs via the same channel
  • we can easily provide our custom implementation

Copy link

github-actions bot commented Oct 9, 2025

⚠️ This PR does not contain any files in the .changes directory.

@pblazej pblazej marked this pull request as ready for review October 13, 2025 08:49
@pblazej
Copy link
Contributor Author

pblazej commented Oct 13, 2025

@hiroshihorie @bcherry I decided to rewrite it from scratch (without waiting for v3), as the previous setup was kinda problematic in multiple ways.

Let me know what you think, especially is there anything "harder to understand than before" for the consumer.

The output looks nice(r) already, can be filtered more easily, etc.:

Screenshot 2025-10-13 at 10 31 25 AM

@hiroshihorie
Copy link
Member

@pblazej Nice, do we have a example app branch for this ? If not I'll make one.

@pblazej
Copy link
Contributor Author

pblazej commented Oct 13, 2025

@hiroshihorie I think these are the only changes you need to make - feel free to test it in spare time 👍

(ofc you can even omit this change)

image


private let minLevel: LogLevel

public init(minLevel: LogLevel = .info, rtc: Bool = false) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The default level is ofc opinionated here, we can go for .warning as well (no consensus on other platforms)... We use these logs sparingly, so for normal connection you just get a couple of messages.

@pblazej pblazej merged commit 7981852 into main Oct 14, 2025
21 of 26 checks passed
@pblazej pblazej deleted the blaze/os-log branch October 14, 2025 07:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants